Skip to content

Clean up created logs on failed markbind serve #2792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
gerteck merged 8 commits into from
Jan 22, 2026
Merged

Clean up created logs on failed markbind serve #2792

gerteck merged 8 commits into from
Jan 22, 2026

Conversation

@Harjun751
Copy link
Contributor

@Harjun751 Harjun751 commented Jan 20, 2026 •
edited by gerteck
Loading

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Resolves #2767

Overview of changes:
Adds a function that cleans up created log directories when attempting to run markbind build or markbind serve in an empty directory. Adds a test for this.

Anything you'd like to highlight/discuss:
I've bumped the memfs devDependency to a newer version since the version we were on did not mock rmSync.

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)

Clean created directories on failed build/serve

Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

At the end of the review, please label the PR with the appropriate label: r.Major, r.Minor, r.Patch.

Breaking change release note preparation (if applicable):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

@codecov
Copy link

codecov bot commented Jan 20, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.73%. Comparing base (f02747b) to head (9f4ddae).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2792      +/-   ##
==========================================
+ Coverage   71.72%   71.73%   +0.01%     
==========================================
  Files         134      134              
  Lines        7299     7303       +4     
  Branches     1556     1613      +57     
==========================================
+ Hits         5235     5239       +4     
  Misses       1937     1937              
  Partials      127      127

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Harjun751 Harjun751 requested a review from Copilot January 20, 2026 06:29
Copilot AI reviewed Jan 20, 2026
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds cleanup functionality to remove the _markbind directory that gets created when markbind build or markbind serve commands fail due to a missing config file in an empty directory. The cleanup prevents polluting empty directories with generated log folders.

Changes:

  • Added cleanupFailedMarkbindBuild() function to remove _markbind directories on failed builds
  • Integrated cleanup calls into build and serve commands when config file is not found
  • Added directory structure comparison capability to test utilities and a test case for empty directory cleanup

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/cli/src/util/cliUtil.js Implements cleanup function to remove _markbind directory
packages/cli/src/cmd/build.js Calls cleanup function when build fails to find config
packages/cli/src/cmd/serve.js Calls cleanup function when serve fails to find config
packages/cli/test/functional/test.js Adds test for empty directory build with cleanup verification
packages/cli/test/functional/testUtil/compare.js Adds directory structure comparison and fixes JSDoc type annotation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Harjun751 Harjun751 marked this pull request as draft January 20, 2026 07:29
@Harjun751 Harjun751 marked this pull request as ready for review January 20, 2026 11:09
MoshiMoshiMochi
MoshiMoshiMochi reviewed Jan 22, 2026
View reviewed changes
Copy link
Contributor

@MoshiMoshiMochi MoshiMoshiMochi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

gerteck
gerteck approved these changes Jan 22, 2026
View reviewed changes
Copy link
Member

@gerteck gerteck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gerteck gerteck merged commit cd1d15e into MarkBind:master Jan 22, 2026
11 checks passed
@github-actions
Copy link

github-actions bot commented Jan 22, 2026

@gerteck Each PR must have a SEMVER impact label, please remember to label the PR properly.

@gerteck gerteck added the r.Patch Version resolver: increment by 0.0.1 label Jan 25, 2026
@gerteck gerteck mentioned this pull request Jan 27, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@gerteck gerteck gerteck approved these changes

+1 more reviewer

@MoshiMoshiMochi MoshiMoshiMochi MoshiMoshiMochi left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

r.Patch Version resolver: increment by 0.0.1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Clean up created logs on failed markbind serve

3 participants

@Harjun751 @MoshiMoshiMochi @gerteck